-
Notifications
You must be signed in to change notification settings - Fork 0
feat: codspeed go add perf v2 support #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds CodSpeed Go performance v2 support by integrating benchmark timing markers and updating the underlying instrument-hooks dependency.
- Adds new benchmark timing functionality using start/end timestamps to track performance measurements
- Updates the instrument-hooks commit to a newer version that supports the v2 performance API
- Introduces new marker types and timing functions for enhanced benchmark instrumentation
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
testing/testing/benchmark.go | Adds startTimestamp field and integrates benchmark timing markers in StartTimer/StopTimer methods |
testing/capi/vendor.sh | Updates instrument-hooks commit hash to newer version with v2 support |
testing/capi/instrument-hooks/includes/core.h | Adds new marker type constants and function declarations for timestamp handling |
testing/capi/instrument-hooks.go | Implements Go bindings for new timestamp and marker functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
7685745
to
b3beed3
Compare
CodSpeed Performance ReportMerging #18 will not alter performanceComparing Summary
Benchmarks breakdown
Footnotes
|
541b758
to
10f3008
Compare
10f3008
to
574f624
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
546ac8a
to
d405bf5
Compare
f5d2d6f
to
c19b2fb
Compare
416dc8a
to
7cd5115
Compare
cc154a3
to
7cd5115
Compare
7cd5115
to
8ee14f6
Compare
@not-matthias don't forget to archive the removed benchmarks since you changed the folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
const ( | ||
MarkerTypeSampleStart = 0 | ||
MarkerTypeSampleEnd = 1 | ||
MarkerTypeBenchmarkStart = 2 | ||
MarkerTypeBenchmarkEnd = 3 | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same feedback as in the Python PR, if there is a way to avoid duplicating this, it would be great
Fixed some unrelated bugs that I discovered while working on this:
./...
) with optimized go-runner (previously it only ran benches in.
with debug go-runner)b.N
loops were slow because we tried to convertbenchTime
(which was already in seconds) to secondsOther changes:
ResetTimer
b.Loop()
due to the extra overhead)